RI-8296 Exact u64 array integer replies (per-command ioredis bigint)#6100
Conversation
🛡️ Jit Security Scan Results✅ No security findings were detected in this PR
Security scan by Jit
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12a8e30808
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Coverage - Backend unit tests
Test suite run success3633 tests passing in 317 suites. Report generated by 🧪jest coverage report action from 481ddc7 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c4390b2. Configure here.
Code Coverage - Integration Tests
|
ce6b4aa to
413f584
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 413f5849a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e333211 to
3a11af7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a11af787d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
96c3d38 to
c73ec0d
Compare
3a11af7 to
5e323a9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4300179f7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4300179 to
cd76ac1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd76ac18db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7db498cd04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
7db498c to
8850b1f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8850b1f558
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…gint [RI-8296]
Adopt the ioredis patch that toggles the parser's stringNumbers per command (via the command queue) and returns BigInt for commands that opt in with { integerReply: 'bigint' } — so other commands are untouched. Opt in the five array integer-reply commands (ARLEN/ARCOUNT/ARNEXT/ARSCAN/ARGREP); their indexes/counts are normalised to strings by toRequiredIndexString. Replaces the earlier global redis-parser approach.
References: #RI-8296
…296]
Detect array u64 integer-reply commands (ARLEN/ARCOUNT/ARNEXT/ARSCAN/ARGREP) in the CLI and request { integerReply: 'bigint' }; the raw/utf-8/text output formatters stringify BigInt so the result isn't rounded and doesn't break JSON serialization.
References: #RI-8296
…set [RI-8296]
Workbench executor opts array u64 integer-reply commands into { integerReply: 'bigint' } and its UTF8/ASCII reply transformers stringify BigInt — same as the CLI. Extract ARRAY_U64_INTEGER_REPLY_COMMANDS into browser-tool-commands.ts as one source of truth for both.
References: #RI-8296
…8296] With the per-command bigint engine, the search endpoint returns the exact u64 index, so the >2^53 round-trip (de-scoped on #6090) is proven again. References: #RI-8296
…8296] main's read-endpoint precision tests use values >= 2^63, which Redis sends as RESP bulk strings (exact without the engine). Add round-trip cases at 2^53+1 — a RESP integer that a JS number rounds — so ARLEN and ARSCAN actually exercise the per-command bigint path. ARCOUNT/ARNEXT can't reach the zone (count/cursor never approach 2^53). References: #RI-8296
Raw-socket probe asserting the actual RESP type byte: a length in (2^53, 2^63) comes back as a RESP integer (':') exact on the wire, and a length >= 2^63 as a bulk string ('$'). Pins the boundary the per-command bigint engine relies on, so a future Redis encoding change trips a test instead of invalidating a comment. Gated to standalone 8.8.
References: #RI-8296
…-8296]
Use requirements('rte.type<>CLUSTER') instead of an in-test this.skip(); the imperative form tripped the strict check tsconfig (TS2683 'this' implicitly any).
References: #RI-8296
Key-info read ARLEN/ARCOUNT via sendPipeline, which can't carry per-command options, so length/count rounded in the (2^53, 2^63) zone. Read them per-command with the bigint opt-in (TTL/MEMORY USAGE stay pipelined for graceful degradation); add a real-server gap-zone integration case. The CLI TEXT formatter recursed array leaves through JSON.stringify, which throws on BigInt, so ARSCAN/ARGREP replies became failed executions. Format BigInt leaves as '(integer) N' like the top level. References: #RI-8296
test.Dockerfile copied patches/ after yarn install, so patch-package found nothing and the ioredis bigint parser never applied — every (2^53, 2^63) array reply rounded in CI. Copy patches/ before install, mirroring the root Dockerfile. Also fix the RESP wire test's cluster gate: conditionalIgnore has no '<>' operator (always skips), so use the supported '!rte.type=CLUSTER'. References: #RI-8296
…RI-8296]
The RAW, ASCII and UTF8 reply formatters stringified bigint integer replies, so the CLI and Workbench rendered u64 array indexes as quoted strings instead of (integer) N. Tag them as { type: 'integer', value } and render the tag in cliTextFormatter, matching redis-cli. Add ARINSERT and ARRING to the u64 integer-reply opt-in.
AROP AND/OR/XOR return a u64 bitwise result that can exceed 2^53; without the per-command bigint opt-in the CLI and Workbench rounded it, breaking the exact-precision path the aggregate response documents. References: #RI-8296
…t [RI-8296] The added bigint test cases reused the untyped `let strategy`, so each new `strategy.format` call raised TS7005 above the recorded baseline (masked locally by a stale incremental tsbuildinfo; CI compiles fresh and fails). Annotate the variable in each spec and refresh the API baseline downward. References: #RI-8296
…-8296]
ArrayService.aggregate sent AROP without the bigint opt-in, so AND/OR/XOR
(and the MATCH/USED counts) above 2^53 rounded before toIndexString saw
them — the Array-tab API returned a rounded value while only Workbench was
exact. Request AROP with { integerReply: 'bigint' } so every operation
stays exact (SUM/MIN/MAX are bulk strings, unaffected), drop the
Workbench-only caveat from the response contract, and add a real-server
gap-zone OR test alongside the SUM one.
References: #RI-8296
ARINFO returns array metadata (length/count/cursor) that for a sparse array can exceed 2^53, but CLI/Workbench left integerReply unset for it, so ioredis rounded the nested integers. Add it to the opt-in set so the metadata renders exact. References: #RI-8296
c6e32ed to
481ddc7
Compare
| // aggregates) and must stay exact above 2^53 — callers request them with | ||
| // { integerReply: 'bigint' }. | ||
| export const ARRAY_U64_INTEGER_REPLY_COMMANDS = new Set<string>([ | ||
| BrowserToolArrayCommands.ArLen, |
There was a problem hiding this comment.
ignore this comment: I believe this follows the names of the original Redis command, but I can't express how I feel when someone saves a few letters to produce ArLen instead of ArrayLength (especially when the other commands below are fully written Next/Scan/Grep/Insert/etc 😀
There was a problem hiding this comment.
it does indeed follow the name of the command - https://redis.io/docs/latest/commands/arlen/
| "src/common/transformers/redis-reply/strategies/ascii-formatter.strategey.spec.ts": { | ||
| "TS7005": 8, | ||
| "TS7034": 2 | ||
| "TS7005": 1, |
…Reply [RI-8296] Replace the any params on the mockCounts helper with the real sendCommand reply type, which already covers the number/bigint/string replies the tests feed it. References: #RI-8296
53be495
…build (#6149) The E2E "Build Docker" job started failing (patch-package: "Failed to apply patch for package ioredis") after RI-8296 (#6100) added patches/ioredis+5.3.2.patch. The patch modified ioredis' built/*.d.ts files. .github/build/build_modules.sh installs the API (patch applies), then runs `yarn autoclean` with .yarnclean.prod — whose `*.ts` rule also deletes `*.d.ts` — and then re-installs, which re-runs patch-package. Because the patch's target .d.ts files were stripped by autoclean, patch-package can no longer apply it and the build fails. (The host workspace build passes because it never autocleans.) The .d.ts changes are type-only and unused by our build: prepareCommandOptions returns `any`, and RedisClient defines its own `integerReply` type — nothing type-checks against ioredis' CommandOptions. So drop the two .d.ts hunks and keep only the runtime Command.js / DataHandler.js changes. patch-package is then idempotent across the autoclean+reinstall cycle. Reproduced the full install -> autoclean -> reinstall sequence locally: the .js-only patch applies cleanly on both passes (`ioredis@5.3.2 ✔`). Co-authored-by: Claude <noreply@anthropic.com>

What
Makes array u64 integer replies exact. RESP encodes
:integers as signedi64, so Redis sends u64 values ≥ 2^63 as bulk strings (exact) but values in
(2^53, 2^63) as RESP integers — which ioredis coerces to a JS number and
rounds above 2^53. That zone was silently lossy for array indexes/lengths.
A patched ioredis
DataHandlertoggles the parser'sstringNumberspercommand via the command queue and returns
BigIntonly for commands thatopt in with
{ integerReply: 'bigint' }. All other commands are untouched.ARRAY_U64_INTEGER_REPLY_COMMANDSset:ARLEN,ARCOUNT,ARNEXT,ARSCAN,ARGREP,ARINSERT,ARRING,AROP,ARINFO.stringify
BigInt.index/length via
toIndexString, andaggregate(AROP) requests bigint soAND/OR/XOR/MATCH/USEDresults stay exact — no BigInt reachesres.json, responses remain decimal strings.ArrayKeyInfoStrategyissuesARLEN/ARCOUNTwith the bigint opt-in ratherthan pipelining them (a pipeline can't carry per-command options).
an experimental env-only strategy with no production caller, so it's left
untouched (bigint there would need RESP3 type mapping — deferred).
Testing
{ integerReply: 'bigint' }opt-in on the arrayservice calls and on the CLI path (including
AROPandARINFO).ARLEN+ARSCAN,the
>2^53ARGREPsearch assertion, and anAROPORaggregate over agap-zone value (run on the 8.8 RTE).
RESP integer (
:), a value ≥ 2^63 as a bulk string ($).CLI
Workbench
Closes #RI-8296
Note
Medium Risk
Touches the patched ioredis transport and all array integer paths; incorrect opt-in or patch application could affect non-array commands or CI if patches are not applied at install time.
Overview
Fixes silent precision loss for Redis array u64 integers in (2^53, 2^63) when ioredis maps RESP integers to JS
number.Adds a patch-package change to ioredis so commands can opt in with
{ integerReply: 'bigint' }; the parser togglesstringNumbersper queued command and converts integer replies toBigInt. The Redis client layer exposesintegerReplyon command options and includesbigintin reply types.Introduces
ARRAY_U64_INTEGER_REPLY_COMMANDSand wires the bigint opt-in through the browser array service (scan, search, length/count/next, aggregate), CLI, and Workbench.ArrayKeyInfoStrategyfetchesARLEN/ARCOUNToutside the pipeline so per-command options apply. Reply formatters (API + CLI) tagbigintas{ type: 'integer', value: '...' }; the UIcliTextFormatterrenders those as(integer) N.Integration and raw-socket tests assert exact strings above
MAX_SAFE_INTEGER; the test Docker image copiespatchesbeforeyarn installso the ioredis patch is applied.Reviewed by Cursor Bugbot for commit 53be495. Bugbot is set up for automated code reviews on this repo. Configure here.